Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Update to new corefx #1561

Merged
merged 1 commit into from
Mar 8, 2018
Merged

Update to new corefx #1561

merged 1 commit into from
Mar 8, 2018

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Mar 8, 2018

No description provided.

Copy link
Contributor

@analogrelay analogrelay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Will this also fix our macOS builds?

@@ -143,7 +144,7 @@ public WebSocketsTransport(WebSocketOptions options, IDuplexPipe application, De
#if NETCOREAPP2_1
var receiveResult = await socket.ReceiveAsync(memory, CancellationToken.None);
#else
var isArray = memory.TryGetArray(out var arraySegment);
var isArray = MemoryMarshal.TryGetArray<byte>(memory, out var arraySegment);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the <byte> not inferrable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first parameter type is ReadOnlyMemory, it's either inference or implicit conversion, unfortunately.

@pakrym
Copy link
Contributor Author

pakrym commented Mar 8, 2018

I hope so, it contains the fix for the issue OSX we reported. We'll see if it works :)

@@ -367,7 +367,7 @@ private static Uri CreateConnectUrl(Uri url, string connectionId)
private async Task StartTransport(Uri connectUrl, TransportType transportType)
{

var options = new PipeOptions(readerScheduler: PipeScheduler.ThreadPool);
var options = new PipeOptions(writerScheduler:PipeScheduler.Inline, readerScheduler: PipeScheduler.ThreadPool, useSynchronizationContext: false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: space after :

@pakrym pakrym force-pushed the feature/pk-corefx0 branch from 6524e37 to 58772ef Compare March 8, 2018 20:56
{
_pipeWriter.Write(source.Span);
_length += source.Length;
return Task.CompletedTask;
return new ValueTask(Task.CompletedTask);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return default;

@@ -20,7 +20,7 @@ public static Task SendAsync(this WebSocket webSocket, ReadOnlySequence<byte> bu
#if NETCOREAPP2_1
if (buffer.IsSingleSegment)
{
return webSocket.SendAsync(buffer.First, webSocketMessageType, endOfMessage: true, cancellationToken);
return webSocket.SendAsync(buffer.First, webSocketMessageType, endOfMessage: true, cancellationToken).AsTask();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this method return ValueTask.

Copy link

@muratg muratg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@davidfowl
Copy link
Member

Super small set of changes.

@pakrym pakrym merged commit 4a4efe0 into dev Mar 8, 2018
@BrennanConroy BrennanConroy deleted the feature/pk-corefx0 branch August 6, 2018 19:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants